PR request 12-15-2018#220
PR request 12-15-2018#220CHENGLIUBI wants to merge 16 commits intoUWPCE-PythonCert-ClassRepos:masterfrom
Conversation
|
Updated mailroom_final.py, mailroom_final_test.py under folder session09 pytest success except add new donor; still need to improve the report & file output pieces. Please review for final grading. I will continue working on it and submit a new PR request when ready. Thank you very much! |
| @@ -0,0 +1,85 @@ | |||
| """ | |||
There was a problem hiding this comment.
you can not have files named """ -- I"m guessing it's a mistake, but make sure not to add them to git!
it's not so bad in OS-X and Linuc, but breaks Windows :-)
please remove this with:
git rm """.py
There was a problem hiding this comment.
Wait! sorry about that, this isn't your file -- it was added by another student -- I had removed it, but not sure how you got it back -- I'll clean it up.
PythonCHB
left a comment
PythonCHB
left a comment
There was a problem hiding this comment.
nice OO structure.
Do make sure that the exception you are catching is actually raised!
| @@ -0,0 +1,85 @@ | |||
| """ | |||
There was a problem hiding this comment.
Wait! sorry about that, this isn't your file -- it was added by another student -- I had removed it, but not sure how you got it back -- I'll clean it up.
|
|
||
|
|
||
| # def test_report(): | ||
| # assert "Jeff Bzo" in report |
| self.donations = list(donations) # converting donations from tuple to list | ||
|
|
||
| @property | ||
| def donor(self): |
There was a problem hiding this comment.
what is this for?? your Donor class holds this info ...
| return (self.name, self.donations) | ||
|
|
||
| @property | ||
| def donor_name(self): |
There was a problem hiding this comment.
why not just use a_donor.name ? The property isn't adding any functionality.
| def list_donations(self): | ||
| try: | ||
| return self.donations | ||
| except IndexError: |
There was a problem hiding this comment.
how would you get an IndexError from returning self.donations ??
| def total_donations(self): | ||
| try: | ||
| return sum(self.donations) | ||
| except IndexError: |
There was a problem hiding this comment.
sum() will not return an IndexError -- it will return zero for an empty list, though :-)
| def num_of_donations(self): | ||
| try: | ||
| return len(self.donations) | ||
| except IndexError: |
There was a problem hiding this comment.
len() wont raise an IndexError either.
|
|
||
| def find_donor(self, name): # find donations based on donor name | ||
| if name in self.donor_data.keys(): | ||
| return self.donor_data.get(name) |
There was a problem hiding this comment.
if you are using .get(), there is not need to check if it's there first.
| return None | ||
|
|
||
| # need more works here | ||
| # def add_donor(self, donors): |
There was a problem hiding this comment.
do you mean this to be:
def add_donor(self, name):
...
|
|
||
| def list_donors(self): | ||
| current_donors = [] | ||
| for data in self.donor_data: # donor_data is a dictionary |
There was a problem hiding this comment.
nice place for a list comprehension here...
| @@ -1,27 +0,0 @@ | |||
|
|
|||
There was a problem hiding this comment.
don't delete anything outside your own students DIR!
you really need to read the git commit messages to make sure that you really want to do what you are doing.
|
I won't merge, unless you restore the file in |
PythonCHB
added
git Problem
3 participants
Updated mailroom oo for final review, but will continue working on it if time allowed. Thank you!